Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add setgroups to std::os::unix::process::CommandExt #72160

Merged
merged 3 commits into from
Jan 22, 2021
Merged

Add setgroups to std::os::unix::process::CommandExt #72160

merged 3 commits into from
Jan 22, 2021

Conversation

slo1
Copy link
Contributor

@slo1 slo1 commented May 13, 2020

Should fix #38527. I'm not sure groups is the greatest name though.

@rust-highfive
Copy link
Collaborator

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2020
@Elinvynia Elinvynia added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 20, 2020
shepmaster
shepmaster reviewed May 23, 2020
View reviewed changes
src/libstd/sys/unix/ext/process.rs Outdated Show resolved Hide resolved
src/libstd/sys/unix/process/process_common.rs Outdated Show resolved Hide resolved
src/libstd/sys/unix/process/process_common.rs Outdated Show resolved Hide resolved
@shepmaster
Copy link
Member

As this appears to add public API (perhaps insta-stable), I'm gonna tag in...

r? @joshtriplett

joshtriplett
joshtriplett requested changes May 23, 2020
View reviewed changes
Copy link
Member

@joshtriplett joshtriplett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some changes; with those made, I'd be happy to re-review.

src/libstd/sys/unix/process/process_common.rs Outdated Show resolved Hide resolved
src/libstd/sys/unix/process/process_common.rs Outdated Show resolved Hide resolved
src/libstd/sys/unix/process/process_unix.rs Outdated Show resolved Hide resolved
src/libstd/sys/unix/process/process_common.rs Outdated Show resolved Hide resolved
@joshtriplett
Copy link
Member

joshtriplett commented May 24, 2020 via email

@slo1 slo1 force-pushed the libstd-setgroups branch 2 times, most recently from 0c426d8 to 9dd239b Compare June 3, 2020 01:56
@stevendanna stevendanna mentioned this pull request Jun 3, 2020
Closed
@Elinvynia Elinvynia added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2020
@slo1
Copy link
Contributor Author

slo1 commented Jun 20, 2020

Sorry for the delay. I made the requested changes

@slo1 slo1 requested a review from joshtriplett June 27, 2020 04:24
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 14, 2020
@JohnCSimon
Copy link
Member

ping from triage:
@slo1 can you mark the change requested by joshtriplett as completed?

@slo1
Copy link
Contributor Author

slo1 commented Jul 14, 2020 •
edited
Loading

ping from triage:
@slo1 can you mark the change requested by joshtriplett as completed?

@JohnCSimon How do I do that? I think I already marked all the conversations as resolved.

@slo1
Copy link
Contributor Author

slo1 commented Jul 22, 2020

@joshtriplett Could you take a look at my changes? Maybe it would have been better to put another commit on top of my previous one, instead of squashing them together.

@bors
Copy link
Contributor

bors commented Jul 28, 2020

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 21, 2021
@slo1 slo1 force-pushed the libstd-setgroups branch 2 times, most recently from d4c1d74 to de36aa0 Compare January 21, 2021 19:57
@KodrAus
Copy link
Contributor

KodrAus commented Jan 22, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jan 22, 2021

📌 Commit de36aa0ed350f7ff6c1df156dd8023cd7f026e7c has been approved by KodrAus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 22, 2021
@bors
Copy link
Contributor

bors commented Jan 22, 2021

⌛ Testing commit de36aa0ed350f7ff6c1df156dd8023cd7f026e7c with merge 31502a228a70000f735f1cfd9a517037686deba5...

@rust-log-analyzer
Copy link
Collaborator

The job dist-various-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 

error: unused variable: `g`
   --> library/std/src/sys/unix/process/process_unix.rs:186:25
    |
186 |             if let Some(g) = self.get_groups() {
    |                         ^ help: if this is intentional, prefix it with an underscore: `_g`
    |
    = note: `-D unused-variables` implied by `-D warnings`
error: aborting due to previous error; 1 warning emitted

[RUSTC-TIMING] std test:false 3.207
error: could not compile `std`
error: could not compile `std`

To learn more, run the command again with --verbose.
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-redox" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "--message-format" "json-render-diagnostics"
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host= --target asmjs-unknown-emscripten,wasm32-unknown-emscripten,x86_64-rumprun-netbsd,mips-unknown-linux-musl,mipsel-unknown-linux-musl,mips64-unknown-linux-muslabi64,mips64el-unknown-linux-muslabi64,arm-unknown-linux-musleabi,arm-unknown-linux-musleabihf,armv5te-unknown-linux-gnueabi,armv5te-unknown-linux-musleabi,armv7-unknown-linux-musleabihf,aarch64-unknown-none,aarch64-unknown-none-softfloat,sparc64-unknown-linux-gnu,x86_64-unknown-redox,thumbv6m-none-eabi,thumbv7m-none-eabi,thumbv7em-none-eabi,thumbv7em-none-eabihf,thumbv8m.base-none-eabi,thumbv8m.main-none-eabi,thumbv8m.main-none-eabihf,riscv32i-unknown-none-elf,riscv32imc-unknown-none-elf,riscv32imac-unknown-none-elf,riscv64imac-unknown-none-elf,riscv64gc-unknown-none-elf,armebv7r-none-eabi,armebv7r-none-eabihf,armv7r-none-eabi,armv7r-none-eabihf,thumbv7neon-unknown-linux-gnueabihf,armv7a-none-eabi
Build completed unsuccessfully in 0:19:11

@bors
Copy link
Contributor

bors commented Jan 22, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 22, 2021
@slo1
Copy link
Contributor Author

slo1 commented Jan 22, 2021

The job dist-various-1 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)

Do I just change g to _g to cover the redox case? I feel that is an awkward fix.

@KodrAus
Copy link
Contributor

KodrAus commented Jan 22, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jan 22, 2021

📌 Commit a4db851 has been approved by KodrAus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 22, 2021
@bors
Copy link
Contributor

bors commented Jan 22, 2021

⌛ Testing commit a4db851 with merge 22ddcd1...

@bors
Copy link
Contributor

bors commented Jan 22, 2021

☀️ Test successful - checks-actions
Approved by: KodrAus
Pushing 22ddcd1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 22, 2021
@bors bors merged commit 22ddcd1 into rust-lang:master Jan 22, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 22, 2021
@joshtriplett joshtriplett mentioned this pull request Feb 18, 2021
Closed
@cuviper cuviper mentioned this pull request Nov 10, 2021
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shepmaster shepmaster shepmaster left review comments

@KodrAus KodrAus KodrAus left review comments

@bjorn3 bjorn3 bjorn3 left review comments

@tmiasko tmiasko tmiasko left review comments

@joshtriplett joshtriplett joshtriplett approved these changes

Assignees

@KodrAus KodrAus

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.51.0
Development

Successfully merging this pull request may close these issues.

feature request: std::os::unix::process::CommandExt.groups()